android: extend support for protocol navigation to Android#815
Conversation
willh-ts
force-pushed
the
will/navigate
branch
2 times, most recently
from
2334b8a to
88f4d5d
Compare
barnstar
left a comment
barnstar
left a comment
There was a problem hiding this comment.
Mainly, I think we need to be really conservative on performing actions with deeplinks (and perhaps audit iOS/macOS if we allow that there). Navigation is perfectly fine, but the idea that clicking a link can change what exit node I'm using has some implications we need to consider before rolling it out.
| } | ||
|
|
||
| private fun presentSettings() { | ||
| popToMain() |
There was a problem hiding this comment.
I think this won't work if main isn't in the backstack, eg if a settings deep link is handled on cold start
There was a problem hiding this comment.
maybe something liek this?
private fun presentSettings() {
navController.navigate("settings") {
popUpTo("main") { inclusive = false }
launchSingleTop = true
}
}
https://developer.android.com/guide/navigation/backstack#pop
| } | ||
|
|
||
| private fun popToMain() { | ||
| navController.popBackStack(route = "main", inclusive = false) |
There was a problem hiding this comment.
maybe check the return value and log if this fails?
| } | ||
| 1 -> { | ||
| pushDeviceDetail(rest[0]) | ||
| true |
There was a problem hiding this comment.
this returns true even if the node is missing, is this intended?
| ?: all.firstOrNull { it.StableID == identifier } | ||
| } | ||
|
|
||
| private fun presentExitNodePicker() { |
There was a problem hiding this comment.
nit: this sounds a little Apple-y. I think Android tends to use "show"
| } | ||
| } | ||
|
|
||
| private fun navigateUri(intent: Intent?): Uri? { |
There was a problem hiding this comment.
maybe rename, since this doesn't actually navigate? this just extracts a uri from the intent, so maybe something liek deepLinkUri for clarity?
| onNavigateHome = backTo("main"), backTo("userSwitcher")) | ||
| } | ||
| } | ||
| if (isIntroScreenViewedSet()) { |
There was a problem hiding this comment.
how does/should this interact with deep linking? eg, if deep linking happens without having viewed the intro screen?
There was a problem hiding this comment.
That's a good call out and I suspect we have the same issue on both iOS and mac. It could make sense in all three cases to gate the DeepLink router on our intro/onboarding properties, ignoring the navigation request if the intro screens haven't yet been marked as seen. I updated MainActivity's pendingDeepLink handler to do that here.
Extends support for the `navigate` path of the `tailscale:` protocol to Android, mirroring the existing macOS routes & pending iOS routes. All settings tabs without existing routes simply open the main settings view for now. ``` tailscale://navigate/main/devices tailscale://navigate/main/devices/<computedName or stableID> tailscale://navigate/main/exit-nodes tailscale://navigate/main/exit-nodes/<stableID> tailscale://navigate/main/exit-nodes/location/<country> tailscale://navigate/settings[/<anyTab>] ``` The `scripts/deeplink-probe.sh` script can be used to trigger the application to navigate to a given route by passing that to it. e.g., `./tool/go run ./scripts/deeplink-probe.go main/devices` updates tailscale/corp#41056 Signed-off-by: Will Hannah <willh@tailscale.com>
3 participants
Extends support for the
navigatepath of thetailscale:protocol to Android,mirroring the existing macOS routes & pending iOS routes.
All settings tabs without existing routes simply open the main settings view
for now.
The
scripts/deeplink-probe.goscript can be used to trigger the application tonavigate to a given route by passing that to it.
e.g.,
./tool/go run ./scripts/deeplink-probe.go main/devicesupdates tailscale/corp#41056